Walkthrough

Adds a GraphQL structural-history schema with manifest, a checker script, and tests; and implements MCP workspace open/list-opened end-to-end (contracts, Zod output schemas, ToolContext wiring, tool definitions, workspace-router tracking/activation, and unit/playback tests).

Changes

Structural History Schema and Validation

MCP Workspace Open / List-Opened

Sequence diagram (MCP workspace open/list flow):

sequenceDiagram
  participant Client
  participant ToolRegistry
  participant WorkspaceOpenTool
  participant WorkspaceRouter
  participant Storage
  Client->>ToolRegistry: invoke workspace_open(cwd, activate?)
  ToolRegistry->>WorkspaceOpenTool: dispatch to handler
  WorkspaceOpenTool->>WorkspaceRouter: ctx.openWorkspace(request)
  WorkspaceRouter->>Storage: resolve workspace & persist openedWorkspaces
  Storage-->>WorkspaceRouter: persistence result
  WorkspaceRouter-->>WorkspaceOpenTool: WorkspaceOpenResult
  WorkspaceOpenTool-->>Client: respond("workspace_open", payload)

🎯 4 (Complex) | ⏱️ ~45 minutes

"🐰 I hopped through types and lines of code,
Manifest clenched tight like a carrot bode,
Routers now track each opened den,
Tools respond and tests prove when."

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

@coderabbitai review please

#!/bin/bash
# Get the PR details and files changed
gh pr view 57 --repo flyingrobots/graft

echo "---FILES CHANGED---"
gh pr diff 57 --repo flyingrobots/graft --name-only

echo "---PR DIFF---"
gh pr diff 57 --repo flyingrobots/graft

@flyingrobots — clean, well-scoped slice. The schema-first approach before wiring Echo is the right sequencing, and the deterministic artifact drift check is a solid foundation. A few things worth looking at:

🔴 Hard-coded wesleyCliVersion check in check-structural-history-schema-artifacts.ts

// scripts/check-structural-history-schema-artifacts.ts  (~line 125)
if (manifest.wesleyCliVersion !== "0.0.4") {
  violations.push(`Unexpected Wesley CLI version ${manifest.wesleyCliVersion}; expected 0.0.4.`);
}

The manifest already carries wesleyCliVersion as the authoritative record. Hard-coding "0.0.4" directly in the script body means every Wesley CLI bump requires a two-file change (manifest + script) and risks the script silently drifting out of sync. The wesleyL1RegistryHash is already the stronger invariant here. Consider either removing this assertion entirely, or deriving the expected version from a constant at the top of the script so the intent is explicit:

const SUPPORTED_WESLEY_CLI_VERSION = "0.0.4";
// ...
if (manifest.wesleyCliVersion !== SUPPORTED_WESLEY_CLI_VERSION) { ... }

That at minimum surfaces the coupling at declaration time rather than buried in the check loop.

🟡 StructuralSourceSpan has no relational constraint on offsets/lines

# schemas/graft-structural-history.graphql
startOffset: Int! `@wes_stateField` `@wes_constraint`(min: 0)
endOffset: Int! `@wes_stateField` `@wes_constraint`(min: 0)
startLine: Int `@wes_stateField` `@wes_constraint`(min: 1)
endLine: Int `@wes_stateField` `@wes_constraint`(min: 1)

Both individual fields are constrained to be ≥ 0 (or ≥ 1), but nothing prevents endOffset < startOffset or endLine < startLine. If Wesley's invariant DSL supports cross-field expressions like the one already used on GraftStructuralHistoryInvariants, a companion invariant (e.g., endOffset >= startOffset) would close this gap before Echo import wires these spans for real.

🟡 StructuralSourceSpan is absent from requiredTypes in the manifest

StructuralSourceSpan is a full @wes_registry(id: 515) type in the schema and is referenced via spanId in both StructuralSymbol and StructuralSymbolRelation, but it does not appear in manifest.requiredTypes. If that's intentional (e.g., span storage is deferred to a later slice), it's worth a brief comment in the manifest to document why.

🟡 Regex built from unsanitized manifest input in typePattern

// scripts/check-structural-history-schema-artifacts.ts
function typePattern(typeName: string): RegExp {
  return new RegExp(`export (interface|type) ${typeName}\\b`, "u");
}

typeName comes from manifest.requiredTypes, which is read from a JSON file. TypeScript type names are alphanumeric so this is safe in practice, but if the manifest were ever tampered with or contained unexpected characters, this would silently break or throw an uncaught SyntaxError during new RegExp(...). A lightweight guard like typeName.replace(/[^A-Za-z0-9_]/g, "") before interpolation would make it resilient.

🟢 Minor nit — _placeholder in GraftStructuralHistoryInvariants

type GraftStructuralHistoryInvariants
  `@wes_invariant`(...) {
  _placeholder: Boolean
}

This is a known GraphQL limitation (a type must have at least one field). A short inline comment explaining the placeholder's purpose would help future readers understand this isn't accidental:

  _placeholder: Boolean  # Required: GraphQL types must declare at least one field.

✅ What's solid

• The manifest + SHA-256 drift check approach is clean and deterministic.
• Excluding src/generated/ from ESLint is the right call.
• The bad-code backlog item (CLEAN_wesley-cli-not-hermetic-in-graft-ci.md) accurately captures the remaining risk and its acceptance criteria are precise.
• Contract tests exercise real behavioral invariants (the FALLBACK_TRANSLATED → nativeContinuumWitness == false rule is encoded both in the schema and in test fixtures), not just structural presence.
• isMainModule() guard using import.meta.url === pathToFileURL(process.argv[1]).href is the correct ESM idiom.

@codex Self-code-review pass against origin/main after a clean worktree check and git fetch origin. No P0/P1 blockers found. I do see several issues worth fixing before merge.

Suggested repair order: #1, #2, #3, then docs/witness cleanup (#4-#8).

To use Codex here, create an environment for this repo.

@codex Follow-up self-review fixes pushed in 61747c5 (fix: resolve self-review findings).

Resolution summary:

Validation run locally:

• pnpm schema:structural-history:check
• pnpm exec vitest run test/unit/contracts/graft-structural-history-schema.test.ts test/unit/mcp/workspace-binding.test.ts test/unit/mcp/opened-workspaces.test.ts tests/playback/SURFACE_opened-workspace-paths.test.ts
• pnpm exec vitest run test/unit/mcp/workspace-binding.test.ts tests/playback/SURFACE_opened-workspace-paths.test.ts
• pnpm lint
• pnpm typecheck
• git diff --check origin/main...HEAD
• pnpm test -> 223 files / 1621 tests passed

To use Codex here, create an environment for this repo.

@codex Self-discovered issue found during closure validation after resolving review threads.

To use Codex here, create an environment for this repo.

@codex Activity Summary for Code Lawyer repair pass.

Validation:

Merge-gate note: checks are green and there are zero Changes Requested reviews, but the PR still has 0 approvals. The gate remains locked until the required 2 approvals arrive.

To use Codex here, create an environment for this repo.